-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add support for native TLS #1354
feat: Add support for native TLS #1354
Conversation
6cfd80c
to
ce257a5
Compare
Hi @mayankshah1607, it seems to be failing because of a conflict with the yaml package. Bumping gojsontoyaml to brancz/gojsontoyaml@202f76b should prevent that. |
If we want to go forward with the TLS configuration from the exporter-toolkit, we might want to document that the configuration can be found here: https://github.com/prometheus/exporter-toolkit/blob/master/docs/web-configuration.md |
c34bbcf
to
183d64b
Compare
Thanks @dgrisonnet. I tried bumping it, but it did not seem to help. Could there be any other reason? |
Hum yes, after taking another look, it seems that I understood the error the other way around. The current |
8695c92
to
79fcb04
Compare
Thanks @dgrisonnet
If everything looks fine in this PR, I could go ahead an add the required documentation. |
@mayankshah1607 Thanks so much for your PR. I have provided my review comments. Let me know if you need any more clarifications around them :) |
79fcb04
to
de4bd9f
Compare
thanks @tariq1890 |
de4bd9f
to
5cd7b99
Compare
So the PR e2e tests are failing because of this statement in the logs
That's a klog error. I think we might need use to klog.Info instead in the new interface implementing method. Also, that's not a well-formed log message . A tad concerning |
- introduce new flag `tls-config` to pass the path to tls config file Signed-off-by: Mayank Shah <mayankshah1614@gmail.com>
5cd7b99
to
ec2a3ff
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mayankshah1607, tariq1890 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
is there a sample --tls-config file ? |
@prasunsultania it's here: https://github.com/prometheus/exporter-toolkit/blob/19e732caf179e5deb553a9cb2bb2da983dd0d830/web/web-config.yml And here is the full config structure as far as I understand: https://github.com/prometheus/exporter-toolkit/blob/19e732caf179e5deb553a9cb2bb2da983dd0d830/web/tls_config.go#L36-L64 |
Fixes #1353
tls-config
to pass the path to TLS config fileSigned-off-by: Mayank Shah mayankshah1614@gmail.com